Add support for negated parameters in SpringMvcContract #1171
Conversation
…cloud#1045) Bumps [com.google.protobuf:protobuf-java](https://github.com/protocolbuffers/protobuf) from 3.25.3 to 3.25.4. - [Release notes](https://github.com/protocolbuffers/protobuf/releases) - [Changelog](https://github.com/protocolbuffers/protobuf/blob/main/protobuf_release.bzl) - [Commits](protocolbuffers/protobuf@v3.25.3...v3.25.4) --- updated-dependencies: - dependency-name: com.google.protobuf:protobuf-java dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…g-cloud#1042) Bumps [com.fasterxml.jackson.dataformat:jackson-dataformat-smile](https://github.com/FasterXML/jackson-dataformats-binary) from 2.17.1 to 2.17.2. - [Commits](FasterXML/jackson-dataformats-binary@jackson-dataformats-binary-2.17.1...jackson-dataformats-binary-2.17.2) --- updated-dependencies: - dependency-name: com.fasterxml.jackson.dataformat:jackson-dataformat-smile dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
# Conflicts: # pom.xml
--- updated-dependencies: - dependency-name: "@antora/collector-extension" dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [@springio/asciidoctor-extensions](https://github.com/spring-io/asciidoctor-extensions) from 1.0.0-alpha.10 to 1.0.0-alpha.13. - [Changelog](https://github.com/spring-io/asciidoctor-extensions/blob/main/CHANGELOG.adoc) - [Commits](spring-io/asciidoctor-extensions@v1.0.0-alpha.10...v1.0.0-alpha.13) --- updated-dependencies: - dependency-name: "@springio/asciidoctor-extensions" dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…d#1063) Bumps [antora](https://gitlab.com/antora/antora) from 3.2.0-alpha.4 to 3.2.0-alpha.6. - [Changelog](https://gitlab.com/antora/antora/blob/main/CHANGELOG.adoc) - [Commits](https://gitlab.com/antora/antora/compare/v3.2.0-alpha.4...v3.2.0-alpha.6) --- updated-dependencies: - dependency-name: antora dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
# Conflicts: # docs/package.json
* Make Content-Encoding configurable A new configuration property was added to make the Content-Encoding configurable when using the feign request compression. The default values are the same as the previous hardcoded values. Closes issue 1048
Bumps commons-io:commons-io from 2.16.1 to 2.17.0. --- updated-dependencies: - dependency-name: commons-io:commons-io dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…cloud#1090) Bumps [com.google.protobuf:protobuf-java](https://github.com/protocolbuffers/protobuf) from 3.25.4 to 3.25.5. - [Release notes](https://github.com/protocolbuffers/protobuf/releases) - [Changelog](https://github.com/protocolbuffers/protobuf/blob/main/protobuf_release.bzl) - [Commits](protocolbuffers/protobuf@v3.25.4...v3.25.5) --- updated-dependencies: - dependency-name: com.google.protobuf:protobuf-java dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
spring-cloud#1087) Bumps [com.google.protobuf:protobuf-java](https://github.com/protocolbuffers/protobuf) from 3.25.4 to 3.25.5. - [Release notes](https://github.com/protocolbuffers/protobuf/releases) - [Changelog](https://github.com/protocolbuffers/protobuf/blob/main/protobuf_release.bzl) - [Commits](protocolbuffers/protobuf@v3.25.4...v3.25.5) --- updated-dependencies: - dependency-name: com.google.protobuf:protobuf-java dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [io.github.openfeign:feign-bom](https://github.com/openfeign/feign) from 13.3 to 13.4. - [Release notes](https://github.com/openfeign/feign/releases) - [Changelog](https://github.com/OpenFeign/feign/blob/master/CHANGELOG.md) - [Commits](OpenFeign/feign@13.3...13.4) --- updated-dependencies: - dependency-name: io.github.openfeign:feign-bom dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* polishing feignClientsRegistrar * update the date in the license and add author
…g-cloud#1097) Bumps [com.fasterxml.jackson.dataformat:jackson-dataformat-smile](https://github.com/FasterXML/jackson-dataformats-binary) from 2.17.2 to 2.18.0. - [Commits](FasterXML/jackson-dataformats-binary@jackson-dataformats-binary-2.17.2...jackson-dataformats-binary-2.18.0) --- updated-dependencies: - dependency-name: com.fasterxml.jackson.dataformat:jackson-dataformat-smile dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
# Conflicts: # pom.xml # spring-cloud-openfeign-dependencies/pom.xml
- Added 'allowNegatedParams' configuration option to FeignClientProperties - Modified SpringMvcContract to skip negated parameters when the feature is enabled - Added tests for both enabled and disabled cases - Updated documentation with usage examples Signed-off-by: kssumin <[email protected]>
kssumin
force-pushed
the
fix/1134-support-negated-params
branch
from
bfa6385 to
849865d
Compare
There was a problem hiding this comment.
Hello @kssumin, thanks for submitting the PR. Have added some comments - please address them. Also, please re-submit your PR against 4.1.x instead of main, so that we can include it in earlier supported release trains as well.
| this.allowNegatedParams = allowNegatedParams; | ||
| } | ||
|
|
||
| public SpringMvcContract(List<AnnotatedParameterProcessor> annotatedParameterProcessors, |
There was a problem hiding this comment.
You should set the allowNegatedParams field from properties here.
...-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java
Show resolved
Hide resolved
...-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java
Show resolved
Hide resolved
...-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java
Show resolved
Hide resolved
...nfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java
Show resolved
Hide resolved
| } | ||
|
|
||
| @Test | ||
| void testAllowNegatedParams() throws Exception { |
There was a problem hiding this comment.
Please use the same naming convention as other tests in this class, starting with should...
| } | ||
|
|
||
| @Test | ||
| void testAllowNegatedParams() throws Exception { |
There was a problem hiding this comment.
Please remove throws Exception
| } | ||
|
|
||
| @Test | ||
| void testDisallowNegatedParams() throws Exception { |
There was a problem hiding this comment.
Please use the same naming convention as other tests in this class, starting with should...,
...n-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java
Show resolved
Hide resolved
| assertThat(data.get(1).bodyIndex()).isEqualTo(0); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
Please add a test where properties are used to change this. This kind of test could be added, for example, to the FeignClientFactoryBeanIntegrationTests class.
|
Fixes gh-1134. |
|
@kssumin the base branch cannot be just changed. The easiest way to correctly change the branch would be to close this PR and create a new one by applying your commits to the |
Got it, I'll close this PR and create a new one based on the 4.1.x branch. |
|
Hello @kssumin, actually I'd really like to ensure that we get this fix into today's |
|
@kssumin, actually, upon further consideration, since this issue is a bug and not an enhancement, I think we should stop throwing the exception altogether - just add some logging and ignore the parameter on Feign contract processing, so I will add that change now instead. While we'll not be including the proposed changes at this point, seeing your PR has helped in deciding on the fix for the issue, so thank you for your work on this issue. |
My full name is Kim Su-min. My surname is Kim.
Thank you for the update. I understand your decision to handle this issue with a logging approach. I'm glad my PR helped in finding the direction for solving the problem. I look forward to actively contributing to the project in the future. |
10 participants
This PR adds support for negated parameters in Spring MVC mappings when used with Feign clients.
It resolves issue #1134 where negated parameters in
@RequestMappingwere not supported, causingIllegalArgumentExceptionwhen shared interfaces were used between server and client.Changes:
allowNegatedParamsconfiguration option toFeignClientPropertiesSpringMvcContractto silently skip negated parameters when this feature is enabledUse case:
When sharing interfaces between server and client code, negated parameters (e.g.
params = {"!expiration"}) are useful on the server side to disambiguate mappings. With this change, clients can now use these shared interfaces by settingspring.cloud.openfeign.client.config.feignName.allowNegatedParams=true.The implementation silently ignores negated parameters when building the client request, which aligns with the expected behavior since negated parameters are constraints used only on the server side.
@OlgaMaciaszek